Overlay V2: fix flooding issues that occasionally cause instability - #5403
Conversation
7d96278 to
0cae17f
Compare
SirTyson
left a comment
There was a problem hiding this comment.
I think there's one potential issue with the way we do peer timeouts (plus one concurrency issue that I don't quite understand that the AI flagged, take it or leave it).
I also think there's a couple places where we could have better tests, but given that this is a dev branch, I also don't care if you ignore those comments either.
There was a problem hiding this comment.
Pull request overview
Fixes Overlay V2 transaction-demand flooding failures and premature transaction-set cache eviction.
Changes:
- Chunks oversized GETDATA messages and improves retry timing.
- Propagates ledger slots through C++/Rust IPC for cache eviction.
- Adds regression tests and adjusts stress-test configuration.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/overlay/test/OverlayIPCTests.cpp |
Updates stress-test capacity and timeout. |
src/overlay/RustOverlayManager.h |
Adds slot parameters to TX-set APIs. |
src/overlay/RustOverlayManager.cpp |
Forwards slots through overlay IPC. |
src/overlay/OverlayIPC.h |
Documents slot-aware IPC APIs. |
src/overlay/OverlayIPC.cpp |
Serializes slots in TX-set messages. |
src/herder/test/HerderTests.cpp |
Reformats disabled test content. |
src/herder/PendingEnvelopes.h |
Removes a blank line. |
src/herder/PendingEnvelopes.cpp |
Supplies the requested TX-set slot. |
src/herder/HerderImpl.cpp |
Supplies locally built TX-set slots. |
overlay/src/xdr.rs |
Exposes the SHA-256 helper. |
overlay/src/main.rs |
Handles slot-aware caching and tests eviction. |
overlay/src/libp2p_overlay.rs |
Implements slot tracking and chunked GETDATA retries. |
overlay/src/ipc/transport.rs |
Updates IPC transport tests. |
overlay/src/ipc/mod.rs |
Exports MessageCodec. |
overlay/src/flood/pending_requests.rs |
Adds post-write timeout stamping. |
overlay/src/flood/inv_messages.rs |
Adds bounded GETDATA chunk encoding. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
c87a7fd to
738e036
Compare
The GETDATA retry path batched every timed-out hash for a peer into a single FloodDemand. TxDemandVector caps at TX_DEMAND_VECTOR_MAX_SIZE (1000) hashes, so under fetch storms the encode failed with 'xdr value max length exceeded' and the entire retry batch was silently dropped, feeding the 30s give-up spiral seen in the 15-node stress test. Add GetData::encode_chunked() which splits hashes into XDR-legal messages, and use it on both the initial and retry send paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
738e036 to
c3242ef
Compare
154f725 to
6b6197d
Compare
|
looks like the flaky test was due to port collision in some tests (cargo test runs them in parallel). should be fixed now, waiting for green CI |
223b48c
into
stellar:overlay-v2-Aug3-rebased
The GETDATA retry path batched every timed-out hash for a peer into a single FloodDemand. TxDemandVector caps at TX_DEMAND_VECTOR_MAX_SIZE (1000) hashes, so under fetch storms the encode failed with 'xdr value max length exceeded' and the entire retry batch was silently dropped, feeding the 30s give-up spiral seen in the 15-node stress test.
Add GetData::encode_chunked() which splits hashes into XDR-legal messages, and use it on both the initial and retry send paths.
Also properly attach ledger_seq to cached tx sets to avoid premature eviction in case of overload.